Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement get_tipset_cid #568

Merged
merged 20 commits into from
Jan 26, 2024
Merged

Implement get_tipset_cid #568

merged 20 commits into from
Jan 26, 2024

Conversation

fridrik01
Copy link
Contributor

@fridrik01 fridrik01 commented Jan 15, 2024

See #508

TODO:

  • Create a new "chain metadata" actor the state data model Akosh suggested in Implement FendermintExterns::get_tipset_cid #508 and using the the suggested interface in the task.
  • Bundle this actor in a separate bundle with its own manifest.
  • Load the new bundle at genesis.
  • Create new actor at genesis, constructing it with a len determined by a code constant.
  • Figure out how to obtain the blockhash at the right time within the ABCI pipeline so we can feed it in (take into account the case where the contract calls BLOCKHASH 0, i.e. the current block, this happens in immediate execution chains like CometBFT/Ignite, unlike Filecoin)
  • Figure out if we need to fork cron, or if we should have a dedicated cron, or if we should call push_blockcid individually.
  • Implement the get_tipset_cid syscall by calling ChainMetadata::blockcid()

Follow up tasks

@fridrik01 fridrik01 force-pushed the chain-metadata-actor branch 2 times, most recently from ebfd26b to 72b8033 Compare January 15, 2024 19:24
@fridrik01 fridrik01 changed the title [WIP] Add chain metadata actor [WIP] Implement get_tipset_cid Jan 16, 2024
@fridrik01 fridrik01 force-pushed the chain-metadata-actor branch 2 times, most recently from 340b854 to 70c8d99 Compare January 16, 2024 17:10
@fridrik01 fridrik01 marked this pull request as ready for review January 18, 2024 19:23
@fridrik01 fridrik01 force-pushed the chain-metadata-actor branch 2 times, most recently from bceae76 to e72eb8c Compare January 22, 2024 10:23
fendermint/actors/build.rs Outdated Show resolved Hide resolved
@fridrik01 fridrik01 changed the title [WIP] Implement get_tipset_cid Implement get_tipset_cid Jan 23, 2024
@fridrik01 fridrik01 requested a review from a team January 23, 2024 18:42
@fridrik01 fridrik01 force-pushed the chain-metadata-actor branch 2 times, most recently from 9963a80 to c34424f Compare January 24, 2024 10:11
//uint256 number = uint256(blockhash(block.number - 1));
bytes32 myHash = blockhash(block.number - 1);

return string(abi.encodePacked(myHash));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail another integration test, namely this one called from here.

I can imagine it has been committed by mistake, but just to clarify, what I thought would be a nice integration test for this is to add a contract very similar to Greeter.sol to testing/contracts that has a function to return the e.g. BLOCKHASH(1), then add an example right next to greeter.rs which is again very similar in that it deploys this contract, then calls it similar to how that one calls greet(), and finally retrieves the block by height to compare the hash against what the contract returned.

The Greeter contract should be left alone to avoid any confusion as it was taken verbatim from another repo where it served the same purpose of testing The Graph.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, this was accidentally committed, it was just what I used for testing

@fridrik01
Copy link
Contributor Author

Working on fixing CI, there was some issue when rebasing, and now getting cargo errors failing to select a version for a package

The fix was to removing "crypto" feature from fvm_shared dependency
in contracts/binding/Cargo.toml, recompile, and then add it back.
# We can work around it by hardcoding the method hashes; currently there is only one.
# frc42_dispatch = "3.2"
# We need a version of frc42_dispatch that has been updated to use FVM 4.1
frc42_dispatch = { git = "https://github.com/fridrik01/filecoin.git", branch = "update-fvm-4.1" }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, that I needed to fork this repo to update it to FVM 4.1. I have a PR for merging this into the official repo (filecoin-project/actors-utils#231)

Copy link
Contributor

@aakoshh aakoshh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I'm not sure why Greeter.bin changes, but as long as CI passes, let's go!

@fridrik01 fridrik01 merged commit 26ff988 into main Jan 26, 2024
20 checks passed
@fridrik01 fridrik01 deleted the chain-metadata-actor branch January 26, 2024 14:53
@raulk raulk linked an issue Jan 26, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement FendermintExterns::get_tipset_cid
2 participants